Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Promise support for http callout #265

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Add Promise support for http callout #265

wants to merge 12 commits into from

Conversation

jizhuozhi
Copy link

@jizhuozhi jizhuozhi commented Oct 11, 2024 •
edited
Loading

In the current envoy WASM plugins, if we need IO request such as HTTP/GRPC/Redis, we must register request to envoy event loop and assigned a token via such as dispatch_http_call and WASM plugin should yield current request lifetime using Action::Pause. When the IO request completed, the envoy will callback to WASM plugin via on_http_call_response, and plugin should dispatch response using token (or ignored if single IO request). If we want to share something between dispatch_http_call and on_http_call_response, we must share them in plugin context fields, it's not a suitable scope.

Here is an example from proxy-wasm-rust-sdk

impl HttpContext for HttpAuthRandom {
    fn on_http_request_headers(&mut self, _: usize, _: bool) -> Action {
        self.dispatch_http_call(
            "httpbin",
            vec![
                (":method", "GET"),
                (":path", "/bytes/1"),
                (":authority", "httpbin.org"),
            ],
            None,
            vec![],
            Duration::from_secs(1),
        )
        .unwrap();
        Action::Pause
    }

    fn on_http_response_headers(&mut self, _: usize, _: bool) -> Action {
        self.set_http_response_header("Powered-By", Some("proxy-wasm"));
        Action::Continue
    }
}

impl Context for HttpAuthRandom {
    fn on_http_call_response(&mut self, _: u32, _: usize, body_size: usize, _: usize) {
        if let Some(body) = self.get_http_call_response_body(0, body_size) {
            if !body.is_empty() && body[0] % 2 == 0 {
                info!("Access granted.");
                self.resume_http_request();
                return;
            }
        }
        info!("Access forbidden.");
        self.send_http_response(
            403,
            vec![("Powered-By", "proxy-wasm")],
            Some(b"Access forbidden.\n"),
        );
    }
}

So there is three major problem we need to resolve:

  1. How to easier dispatch request and callback response via token.
  2. How to share something between dispatch request and callback response with smaller scope.
  3. How to make code more fluid instead of spreading logic in different places

In Rust async programming, normally we use async/await for IO request, but in envoy WASM plugin, there is no executor to poll future. A suitable solution is providing JavaScript style Promise (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise). With Promise, we could write all logic in single function

impl HttpContext for HttpAuthRandom {
    fn on_http_request_headers(&mut self, _: usize, _: bool) -> Action {
        let token = self.dispatch_http_call(
            "httpbin",
            vec![
                (":method", "GET"),
                (":path", "/bytes/1"),
                (":authority", "httpbin.org"),
            ],
            None,
            vec![],
            Duration::from_secs(1),
        )
        .unwrap();
        let promise = Promise::new();
        // make relation between promise and request (token)
        promise.then(|(_, _, body_size, _)| {
            if let Some(body) = hostcalls::get_http_call_response_body(0, body_size) {
                if !body.is_empty() && body[0] % 2 == 0 {
                    info!("Access granted.");
                    hostcalls::resume_http_request();
                    return;
                }
            }
            info!("Access forbidden.");
            hostcalls::send_http_response(
                403,
                vec![("Powered-By", "proxy-wasm")],
                Some(b"Access forbidden.\n"),
            );
        })
        Action::Pause
    }

    fn on_http_response_headers(&mut self, _: usize, _: bool) -> Action {
        self.set_http_response_header("Powered-By", Some("proxy-wasm"));
        Action::Continue
    }
}

It seems more fluid then writing in callback of on_http_call_response, but there is no executor for promise to trigger state transferring. We can use on_http_call_response as trigger simply

struct HttpAuthRandom {
    promise: Rc<Promise<(u32, usize, usize, usize)>>
}

impl HttpContext for HttpAuthRandom {
    fn on_http_request_headers(&mut self, _: usize, _: bool) -> Action {
      // ...
      // make relation between promise and request (token)
      self.promise = promise.clone();
      // ...
    }
}

impl Context for HttpAuthRandom {
    fn on_http_call_response(&mut self, _token_id: u32, _num_headers: usize, _body_size: usize, _num_trailers: usize) {
        self.promise.fulfill((_token_id, _num_headers, _body_size, _num_trailers))
    }
}

As for making relationship between multi tokens and promises, we could just using HashMap with insert/remove (maybe it should be embed in SDK but not belongs to this PR)

struct HttpAuthRandom {
    m: HashMap<u32, Rc<Promise<(u32, usize, usize, usize)>>
}

impl HttpContext for HttpAuthRandom {
    fn on_http_request_headers(&mut self, _: usize, _: bool) -> Action {
      // ...
      // make relation between promise and request (token)
      self.m.insert(token, promise.clone());
      // ...
    }
}

impl Context for HttpAuthRandom {
    fn on_http_call_response(&mut self, _token_id: u32, _num_headers: usize, _body_size: usize, _num_trailers: usize) {
        let promise = self.m.remove(token);
        promise.fulfill((_token_id, _num_headers, _body_size, _num_trailers))
    }
}

Note: hostcalls is re-implemented of resume_http_request and send_http_response because we cannot move self to callbacks.

@jizhuozhi jizhuozhi mentioned this pull request Oct 13, 2024
Open
@jizhuozhi
Add Promise support for http callout
cfefbcf 
fix ci

Signed-off-by: jizhuozhi.george <[email protected]>
@jizhuozhi
Add Promise support for http callout
a1e4f9e 
ci add `http_parallel_call`

Signed-off-by: jizhuozhi.george <[email protected]>
@jizhuozhi
Copy link
Author

jizhuozhi commented Oct 14, 2024 •
edited
Loading

msrv/stable/nightly will fix, but another outdated/reactors seems not belong to this PR, PTAL :)

@jizhuozhi
Add Promise support for http callout
b4150bc 
fix ci

Signed-off-by: jizhuozhi.george <[email protected]>
PiotrSikora
PiotrSikora reviewed Oct 14, 2024
View reviewed changes
examples/http_parallel_call/Cargo.toml Outdated Show resolved Hide resolved
examples/http_parallel_call/src/lib.rs Outdated Show resolved Hide resolved
examples/http_parallel_call/src/lib.rs Outdated Show resolved Hide resolved
examples/http_parallel_call/src/lib.rs Outdated Show resolved Hide resolved
examples/http_parallel_call/src/lib.rs Outdated Show resolved Hide resolved
src/promise.rs Outdated Show resolved Hide resolved
src/promise.rs Outdated Show resolved Hide resolved
@PiotrSikora
Copy link
Member

msrv/stable/nightly will fix, but another outdated/reactors seems not belong to this PR, PTAL :)

Thanks! I'll fix existing errors shortly. Sorry about that!

@jizhuozhi
Add Promise support for http callout
de19425 
move promise and dispatch http request to callout folder

Signed-off-by: jizhuozhi.george <[email protected]>
@jizhuozhi
Add Promise support for http callout
5c95bb4 
remove Chinese comments

Signed-off-by: jizhuozhi.george <[email protected]>
@jizhuozhi
Copy link
Author

There is currently a problem involving the collaboration between SDK and user code: HttpClient is created by user code, but the callback is driven by the SDK. There is currently no way to shield this detail within the SDK. User code always needs to write boilerplate code in on_http_call_response to driver callback.

I have no idea to resolve this problem :(

jizhuozhi and others added 5 commits October 14, 2024 17:07
@jizhuozhi
Add Promise support for http callout
95f4d06 
simplify http_parallel_call example

Signed-off-by: jizhuozhi.george <[email protected]>
@jizhuozhi
Add Promise support for http callout
a5ff216 
fix ci

Signed-off-by: jizhuozhi.george <[email protected]>
@jizhuozhi
Add Promise support for http callout
dc6e39f 
fix example cloning self to move but not using hostcalls

Signed-off-by: jizhuozhi.george <[email protected]>
@jizhuozhi
Add Promise support for http callout
d797999 
fix licenses and bazel (maybe)

Signed-off-by: jizhuozhi.george <[email protected]>
@PiotrSikora
Merge branch 'main' into main
fdc72a6
@jizhuozhi
Copy link
Author

jizhuozhi commented Nov 3, 2024 •
edited
Loading

Bazel BUILD file has been reformatted :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PiotrSikora PiotrSikora PiotrSikora left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jizhuozhi @PiotrSikora